Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

creation of car from file / directory #246

Merged
merged 16 commits into from
Oct 21, 2021
Merged

creation of car from file / directory #246

merged 16 commits into from
Oct 21, 2021

Conversation

willscott
Copy link
Member

@willscott willscott commented Sep 29, 2021

  • car create -f out.car [file|dir]...
  • car list --unixfs unixfs.car
  • car list -v any.car
  • car index --v1 any.carv2

Depends on ipfs/go-unixfsnode#12

@mvdan
Copy link
Contributor

mvdan commented Sep 30, 2021

I pushed an initial test script to make reproing easier.

@willscott
Copy link
Member Author

FYI you can do both files and directories fairly easily with filepath.WalkDir

Makes sense. I'm building the unixfsnode builders for files and then directories at the same time, which is why these look a bit piecemeal.

@rvagg
Copy link
Member

rvagg commented Oct 15, 2021

@willscott from what I can see, this is doing a leaves-first build of the DAG, right? There's no fancy buffering or anything else going on to order it according to a top-down DAG walk. Which I think gives it parity with ipfs-car (JS) but isn't the "deterministic" ordering you'd get from ipfs dag export and a filesystem Lotus import (I'm not suggesting this is a requirement, just want to be absolutely clear that I'm not missing any magic here).

@willscott
Copy link
Member Author

correct. this is write order, and does not follow any specific selector order.

cmd/car/car.go Outdated
Comment on lines 25 to 26
Name: "file",
Aliases: []string{"f"},
Copy link
Member

@olizilla olizilla Oct 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-o --output would be mildly more famililar here and consistent with ipfs-car

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i was trying to follow the flags from tar, i'll add both as equivalent aliases.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, this is for creating a car not extracting. Ignore me. As you were.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tho, on second thoughts it does still feel more like an --output kind of a deal here. ipfs-car is using --output for "where to write the car to" when creating and "where to unpack the files to" when extracting.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Déjà vu! Oli and I had identical argument regarding ipfs-car, I said be like tar, Oli says tar is terrible, don't copy it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uh huh. "if you know how tar works then you know how car works" sounds compelling, and given our erudite nieche may be the right choice for the immediate audience.

My experience has been that tar is used as an example of a command that folks stuggle to remember what the flags do, so I aimed elsewhere. But we can have both. ipfs-car is not car and won't try to be, and folks can use the flavour that works for them.

@willscott
Copy link
Member Author

@rvagg when i go to car.ipfs.io for a file, i get the file metadata and directory (the root block) at the end of the car - the same that happens with this command currently.

@willscott willscott marked this pull request as ready for review October 19, 2021 18:33
@willscott
Copy link
Member Author

If i don't hear objections to the current state of this PR in the next day i'm going to move forward merging it, and use smaller follow-up PRs to improve on remaining rough edges

@warpfork
Copy link
Contributor

No objections!

@rvagg
Copy link
Member

rvagg commented Oct 21, 2021

oh, @willscott you'll need to fix up get while you're in here with a newer go-ipld-prime, this broke some time ago and should probably be addressed:

diff hidden ```diff diff --git a/cmd/car/get.go b/cmd/car/get.go index 0343154..8f81b1e 100644 --- a/cmd/car/get.go +++ b/cmd/car/get.go @@ -87,7 +87,7 @@ func GetCarDag(c *cli.Context) error { ls := cidlink.DefaultLinkSystem() ls.TrustedStorage = true ls.StorageReadOpener = func(_ linking.LinkContext, l datamodel.Link) (io.Reader, error) { - if cl, ok := l.(*cidlink.Link); ok { + if cl, ok := l.(cidlink.Link); ok { blk, err := bs.Get(cl.Cid) if err != nil { if err == ipfsbs.ErrNotFound { @@ -105,7 +105,7 @@ func GetCarDag(c *cli.Context) error { ls.StorageWriteOpener = func(_ linking.LinkContext) (io.Writer, linking.BlockWriteCommitter, error) { buf := bytes.NewBuffer(nil) return buf, func(l datamodel.Link) error { - if cl, ok := l.(*cidlink.Link); ok { + if cl, ok := l.(cidlink.Link); ok { blk, err := blocks.NewBlockWithCid(buf.Bytes(), cl.Cid) if err != nil { return err ```

@rvagg
Copy link
Member

rvagg commented Oct 21, 2021

never mind, I pushed that change, I was in the code anyway

@willscott willscott merged commit 9bd7416 into master Oct 21, 2021
@willscott willscott deleted the unixfscreate branch October 21, 2021 18:14
rvagg added a commit that referenced this pull request Oct 22, 2021
* creation of car from file / directory

Co-authored-by: Daniel Martí <mvdan@mvdan.cc>
Co-authored-by: Rod Vagg <rod@vagg.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants